Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing ambiguous input with a timezone (Redux) #93

Merged
merged 1 commit into from
Jun 19, 2014

Conversation

timrwood
Copy link
Member

This is a rebase of the tests from #27 on top of the new architecture.

When DST ends, there is an hour that is duplicated.

At the end of DST in the US, at 2am the clocks roll back an hour to 1am. Thus, given a time from 1am to 2am, it could be either before or after the rollback. The time progression is 12:00..12:59 1:00..1:59 1:00..1:59 2:00.

This is easier to understand with the offsets noted.

12:00 +07:00 = 07:00
01:00 +07:00 = 08:00
01:00 +08:00 = 09:00
02:00 +08:00 = 10:00

When DST starts, there is an hour that is lost.

At the start of DST in the US, at 2am the clocks roll forward an hour to 3am. Thus, there is no such time between 2am and 3am. The time progression is 12:00..12:59 1:00..1:59 3:00.

12:00 +08:00 = 08:00
01:00 +08:00 = 09:00
03:00 +07:00 = 10:00
04:00 +07:00 = 11:00

The assumption made in the tests below is to always 'round down'.

When DST is ending, if there are two valid times, use the one with the lower offset. This has the fault of not being able to parse 01:00+08:00.

12:00 -> 12:00 +07:00
01:00 -> 01:00 +07:00
02:00 -> 02:00 +08:00

When DST is starting, if there is no valid time, subtract the DST change (typically one hour) and use that value. This has the fault of changing the value of 02:00 to 01:00.

12:00 -> 12:00 +08:00
01:00 -> 01:00 +08:00
02:00 -> 01:00 +08:00
03:00 -> 03:00 +07:00

TODO:

  • Add tests for Zone#parse.
  • Add tests to make sure offsets are only being added if moment.tz.needsOffset is true.

Add moment.tz.needsOffset to check if a newly created moment was parsed without an offset.
@stevehof
Copy link

Thanks, I'll run some tests and let you know what i find.

@timrwood
Copy link
Member Author

Merged this into develop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants